Skip to content

Conversation

@anoopkg6
Copy link
Contributor

Fix endianness issue with MachO_ptrauth_noolloc_sections.yaml relocations. Semantics of the r_word1 field is different depending on the endianness.

@anoopkg6 anoopkg6 changed the title Fix endiannes issue with MachO_ptrauth_noolloc_sections.yaml [JITLINK]Fix endiannes issue with MachO_ptrauth_noolloc_sections.yaml Nov 13, 2025
@uweigand
Copy link
Member

@lhames after enabling JITLink tests on SystemZ, one MachO test started failing. It looks like JITLink doesn't take relocation endianness into account like the main MachO code does - can you have a look at this PR?

@lhames
Copy link
Contributor

lhames commented Nov 14, 2025

@uweigand This is unexpected:

MachO::any_relocation_info ARI =
        getObject().getRelocation(RelItr->getRawDataRefImpl());
    MachO::relocation_info RI;

getRelocation() should be returning an any_relocation_info that has already been swapped into the native endianness (by the call to getStruct here:

return getStruct<MachO::any_relocation_info>(
, with getStruct performing the swap here:
if (O.isLittleEndian() != sys::IsLittleEndianHost)
). If this endianness swap wasn't kicking in then I'd expect many more tests to fail.

Given that it is a yaml test that is failing I suspect that the endianness issue is in the MachO YAML reader / writer.

@uweigand
Copy link
Member

@lhames , yes, a byte-swap is already happening. However, there's apparently also an endian-dependent bitfield difference that is not handled automatically. If you look at the full code in MachOObjectFile.cpp, you see e.g.

uint64_t MachOObjectFile::getRelocationType(DataRefImpl Rel) const {
  MachO::any_relocation_info RE = getRelocation(Rel);
  return getAnyRelocationType(RE);
}

This calls getRelocation, which performs the byte swap. But then

unsigned
MachOObjectFile::getAnyRelocationType(
                                   const MachO::any_relocation_info &RE) const {
  if (isRelocationScattered(RE))
    return getScatteredRelocationType(RE);
  return getPlainRelocationType(*this, RE);
} 

calls

static unsigned getPlainRelocationType(const MachOObjectFile &O,
                                       const MachO::any_relocation_info &RE) { 
  if (O.isLittleEndian())
    return RE.r_word1 >> 28;
  return RE.r_word1 & 0xf;
} 

which gets different bits depending on target endianness.

This latter part is not currently not by the JITLink code.

@lhames
Copy link
Contributor

lhames commented Nov 17, 2025

@lhames , yes, a byte-swap is already happening. However, there's apparently also an endian-dependent bitfield difference that is not handled automatically.

Oh, of course! Thanks for pointing that out.

I want to avoid any external calls and further dependence on MachOObjectFile here. Does #168304 solve the issue for you? If so we can go for that instead.

@lhames
Copy link
Contributor

lhames commented Nov 17, 2025

Scratch that -- I just got LLVM building on ppc64be to test this out and my "fix" breaks other test cases. I should have an update shortly.

lhames added a commit to lhames/llvm-project that referenced this pull request Nov 17, 2025
The "MachO_ptrauth_noolloc_sections.yaml" testcase had a typo in the name, and
didn't explicitly specify its endianness. This was causing it to fail when
enabled on big endian platforms.

See conversation in llvm#167902
@lhames
Copy link
Contributor

lhames commented Nov 17, 2025

Previous PR was a bum steer. I believe that the "bitfields" in this struct have the same layout in both endiannesses (see comment at https://github.com/apple-oss-distributions/cctools/blob/920a2b45080fb9badf31bf675f03b19973f0dd4f/include/mach-o/reloc.h#L140). I think the real fix for this is #168323

lhames added a commit that referenced this pull request Nov 17, 2025
…168323)

The "MachO_ptrauth_noolloc_sections.yaml" testcase had a typo in the
name, and didn't explicitly specify its endianness. This was causing it
to fail when enabled on big endian platforms.

See conversation in #167902
@lhames
Copy link
Contributor

lhames commented Nov 17, 2025

#168323 has been merged. If that fixes your issue then I think that we can close this.

llvm-sync bot pushed a commit to arm/arm-toolchain that referenced this pull request Nov 17, 2025
…testcase. (#168323)

The "MachO_ptrauth_noolloc_sections.yaml" testcase had a typo in the
name, and didn't explicitly specify its endianness. This was causing it
to fail when enabled on big endian platforms.

See conversation in llvm/llvm-project#167902
@uweigand
Copy link
Member

#168323 has been merged. If that fixes your issue then I think that we can close this.

Yes, this does indeed fix the issue for us. Thanks!

@uweigand uweigand closed this Nov 17, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants